fix(android): correctly end view transitions #3250

hannojg · 2025-09-25T09:39:49Z

Description

Note

This PR still contains the reproduction code necessary + it contains changes related to RN core. This is meant so we can have a conversation about this issue and see how to apply the fixes best.

There are two problems which this PR tries to face to fix usage with recycling views:

1. `Screen#endRemovalTransition` is broken:

react-native-screens/android/src/main/java/com/swmansion/rnscreens/Screen.kt

Lines 472 to 477 in 4982d4c

    
               endTransitionRecursive(this) 
        
           } 
        
           private fun endTransitionRecursive(parent: ViewGroup) { 
        
               parent.children.forEach { childView -> 
        
                   parent.endViewTransition(childView)

Here you can see that we try to iterate over the children of the screen to end the view transition. The problem is that the react-native mounting layer already has all the children removed from the parent. The flow of events is:

The user requests to remove a screen, we call through NativeProxy startRemovalTransition. All children will be marked as "in-transition"
In JS react our screen component gets removed (i believe thats how its working, I might be wrong) and the react-native mounting layer will call removeView for all the children of our screen
The views will remain visible as we marked them as "in transition". HOWEVER, the children will already get removed from the parents mChildren list. Only the children will keep their mParent field set
We call endRemovalTransition once the transition is done, trying to iterate over the children, but they are already unset. You can verify this by logging the children count when this method gets called
🐛 Bug: Our views, that may get recycled, are never called with endViewTransition(child) and their mParent field never gets reset. Thus the react-native mounting layer will always crash when trying to reuse this view.

2. Interrupting a screen transition can cause a wrong order of events

We rely on Screen#endRemovalTransition to be called before ScreenStack#endRemovalTransition. ScreenStack is in some way the parent of Screen. So the parent of the screen first calls endViewTransition. This is problematic because:

When a parent gets called with endViewTransition it will loop over all children and call dispatchDetachedFromWindow():
- ViewGroup#endViewTransiton
- ViewGroup#dispatchDetachedFromWindow
The problem with that is that the children will be simply detached, and cleared from the mDisappearingChildren
Later, when we call Screen#endRemovalTransition and loop over the views calling endViewTransition will have no effect, since the views are no longer marked as disappearing:
- https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/core/java/android/view/ViewGroup.java;l=7229-7233
🐛 Bug: Due to this the children's mParent field never gets reset. Thus the react-native mounting layer will always crash when trying to reuse this view.

Changes

1. Fixing `Screen#endRemovalTransition` is broken:

I simply added a list of parent-child pairs that we add to in startRemovalTransition. Then when endRemovalTransition gets called we loop over that list.

2. Fixing interrupting a screen transition can cause a wrong order of events:

Two fixes are necessary for this:

In ScreenStack we call Screen#endViewTransition first, before calling into super.endViewTransition. I think this is safe since Screen#endRemovalTransition has a flag checking if we are currently in a removal transition or not
We need to call endViewTransition from the most inner children up to the top parent so we avoid the problem explained above, where calling endViewTransition on a parent will remove all children from mDisappearingChildren list

Additional changes in react-native core

As you can see I patched RN + override the default ReactViewManager. I think we also need to address this in react-native somehow. There are two things that need to be addressed I believe:

When recycling views (onDropViewInstance) we could check if a view still has a parent. If so we could assume its in transition (or maybe have it marked as such earlier), and only add the view to the recyling pool on detach (ie. when the transition has ended)
This crash seems not just reproducible with recycling views. It was just one way for me to get it to reproduce, but it seems that there might be other code paths where the same native view is trying to be reused (at least thats what the crash report's telemetry suggest me) [also in the app where this is happening we don't have view recycling enabled]. So additionally we might want to add something to addView in the ReactViewManager / BaseViewManager that will check if a view is "in-transition" / has a parent, and only attach the view once the transition is done (ie. on detach)

➤ I think that we can still apply the changes in RNS without needing to wait for react-native core though.

Screenshots / GIFs

Before	After
Screen_Recording_20250924_151216_FabricExample.mp4	Screen_Recording_20250925_102125_FabricExample.mp4

Test code and steps to reproduce

You can simply run this PR and go to the TestRecylingViews tab in the example app to play with it.

Checklist

Included code example that can be used to test this change
Updated TS types
Updated documentation:
Ensured that CI passes

kkafar

Hey, this looks really sensible.

The views will remain visible as we marked them as "in transition". HOWEVER, the children will already get removed from the parents mChildren list. Only the children will keep their mParent field set
We call endRemovalTransition once the transition is done, trying to iterate over the children, but they are already unset. You can verify this by logging the children count when this method gets called

You're exactly right here. A while ago I fixed similar problem in core, yet here I've failed to notice this 😅

Therefore I'm all for landing part of this patch that applies to screens incorrectly ending view transition.

As for the view recycling part, I'll admit that we have not considered up to this moment much, as, correct me if I'm wrong, it is disabled by default on Android.
I would like to avoid (if possible) code for special handling of view recycling in screens. I'd just prefer to fix it in core.
So thanks to my PR to core, the ReactViewGroup tracks which children have been removed during transition and which were not -> this is information we could inspect from child view to make sure, we've been removed from parent while being in transition, or we can add similar bit of logic to ReactViewGroupManager (as also you suggest) - check whether the view has parent or not. This is the best method I've found back then.

[...] but it seems that there might be other code paths where the same native view is trying to be reused

Yeah, that can happen. In screens we've gone a while ago with "recycle method" that solved this for screen instances. Might be reasonable to also upstream it to core. But I'd be more in favour of identifying particular code paths & culprits that lead to those kind of crashes instead of "recycling" every view in core, as it would only encourage incorrect code from libraries side.

hannojg · 2025-10-10T12:35:40Z

As for the view recycling part, I'll admit that we have not considered up to this moment much, as, correct me if I'm wrong, it is disabled by default on Android.

yes thats correct, also not sure if this is ever gonna land in stable 😅 It was just one way for me to reproduce this crash.

ReactViewGroup tracks which children have been removed during transition and which were not -> this is information we could inspect from child view to make sure, we've been removed from parent while being in transition

Ah yeah, thats nice, i will experiment with that!

Okay cool, so then i'd remove everything else which i added for reproduction from this PR and we can move forward with the main change? 😊

kkafar · 2025-10-13T11:25:27Z

Yes, we can. Let's get this PR up to date & I think we can proceed.

hannojg · 2025-10-13T13:40:38Z

Cool, there is just one thing that might needs to be changed. I had this change in production and saw today the following reports in sentry:

java.util.ConcurrentModificationException: null
    at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1111)
    at java.util.ArrayList$ListItr.previous(ArrayList.java:1138)
    at kotlin.collections.ReversedList$listIterator$1.next(ReversedViews.kt:48)
    at com.discord.view.ScreenOverride.endRemovalTransition(ScreenViewManagerOverride.kt:96)
    at com.swmansion.rnscreens.ScreenStackFragment.onViewAnimationEnd(ScreenStackFragment.kt:141)
    at com.swmansion.rnscreens.stack.views.ScreensCoordinatorLayout$animationListener$1.onAnimationEnd(ScreensCoordinatorLayout.kt:38)
    at android.view.animation.Animation.dispatchAnimationEnd(Animation.java:1171)
    at android.view.animation.AnimationSet.getTransformation(AnimationSet.java:418)
    at android.view.animation.Animation.getTransformation(Animation.java:1190)
    at android.view.View.applyLegacyAnimation(View.java:25871)
    at android.view.View.draw(View.java:26010)
    at android.view.ViewGroup.drawChild(ViewGroup.java:4810)
    at com.swmansion.rnscreens.ScreenStack.performDraw(ScreenStack.kt:335)
    at com.swmansion.rnscreens.ScreenStack.access$performDraw(ScreenStack.kt:20)
    at com.swmansion.rnscreens.ScreenStack$DrawingOp.draw(ScreenStack.kt:348)
    at com.swmansion.rnscreens.ScreenStack.drawAndRelease(ScreenStack.kt:304)
    at com.swmansion.rnscreens.ScreenStack.dispatchDraw(ScreenStack.kt:314)
    at android.view.View.updateDisplayListIfDirty(View.java:25161)
    at android.view.ViewGroup.recreateChildDisplayList(ViewGroup.java:4794)
    ...

I can see from the breadcrumbs, that this crash happens, while we are still executing startRemovalTransition (there is only a 9ms difference between the two calls).
I think the problem here is that startRemovalTransition can be called from the JS thread, setting isBeingRemoved = true. Then on the UI thread the animation is very quickly done , and already calling endRemovalTransition, concurrently modifying the intransitionViews list.

My fix for this right now would be to always run startRemovalTransition from the UI thread. What do you think about that change?
Would look something like:

    // Note: this can be called from the JS thread 👀
   fun startRemovalTransition() {
        // Dispatch on the main thread if not already on it
        val mainLooper = Looper.getMainLooper()
        if (Looper.myLooper() != mainLooper) {
            Handler(mainLooper).post {
                startRemovalTransition()
            }
            return
        }

        if (isBeingRemoved) return
        isBeingRemoved = true

        startTransitionRecursive(this)
    }

kkafar · 2025-10-13T14:03:52Z

I see.
The problem here is that we had at least two attempts to schedule the call to startRemovalTransition on UI & we backed out. I'll try to find the old PRs

kkafar · 2025-10-13T14:07:16Z

We have this one here: #2964
& it seems I remembered things wrong. We just didn't finalise the PR.

I'll see to that one first & see whether we can land it before we land this one.

hannojg added 2 commits September 24, 2025 15:21

reproduction for crash

d5d9df2

reproduction enabling recycling + potential fix

99ac08f

hannojg mentioned this pull request Sep 25, 2025

View recycling breaks due to fragment view transitions #3249

Open

kkafar reviewed Oct 8, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(android): correctly end view transitions #3250

fix(android): correctly end view transitions #3250

Uh oh!

hannojg commented Sep 25, 2025 •

edited

Loading

Uh oh!

kkafar left a comment •

edited

Loading

Uh oh!

hannojg commented Oct 10, 2025

Uh oh!

kkafar commented Oct 13, 2025

Uh oh!

hannojg commented Oct 13, 2025 •

edited

Loading

Uh oh!

kkafar commented Oct 13, 2025

Uh oh!

kkafar commented Oct 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	endTransitionRecursive(this)
	}

	private fun endTransitionRecursive(parent: ViewGroup) {
	parent.children.forEach { childView ->
	parent.endViewTransition(childView)

Uh oh!

fix(android): correctly end view transitions #3250

Are you sure you want to change the base?

fix(android): correctly end view transitions #3250

Uh oh!

Conversation

hannojg commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

1. Screen#endRemovalTransition is broken:

2. Interrupting a screen transition can cause a wrong order of events

Changes

1. Fixing Screen#endRemovalTransition is broken:

2. Fixing interrupting a screen transition can cause a wrong order of events:

Additional changes in react-native core

Screenshots / GIFs

Test code and steps to reproduce

Checklist

Uh oh!

kkafar left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hannojg commented Oct 10, 2025

Uh oh!

kkafar commented Oct 13, 2025

Uh oh!

hannojg commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kkafar commented Oct 13, 2025

Uh oh!

kkafar commented Oct 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hannojg commented Sep 25, 2025 •

edited

Loading

1. `Screen#endRemovalTransition` is broken:

1. Fixing `Screen#endRemovalTransition` is broken:

kkafar left a comment •

edited

Loading

hannojg commented Oct 13, 2025 •

edited

Loading